Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add GROUND_DISCONNECTOR to existing kinds of Switch #2757

Closed
wants to merge 2 commits into from
Closed

Add GROUND_DISCONNECTOR to existing kinds of Switch #2757

wants to merge 2 commits into from

Conversation

So-Fras
Copy link
Member

@So-Fras So-Fras commented Oct 24, 2023

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?
#1131
But, in fact, ground disconnectors are not ignored but classified as disconnectors.

What kind of change does this PR introduce?
Bug fix

What is the current behavior?
No distinction is made between ground disconnectors and disconnectors in IIDM.

What is the new behavior (if this is a feature change)?
Ground disconnectors and disconnectors are distinct.
Moreover, no internal connection should be added to the node of the ground disconnector that is connected "to the ground".

Does this PR introduce a breaking change or deprecate an API?

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

@So-Fras
First addings of GROUND_DISCONNECTOR
afd4ea8 
Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>
@geofjamg
Copy link
Member

I don't understand why we add a new type of switch. Ground disconnectors are just disconnectors and grounding is related to the way it is connected. Nothing related to a new switch type.

@So-Fras So-Fras mentioned this pull request Oct 24, 2023
Merged
5 tasks
geofjamg
geofjamg requested changes Oct 24, 2023
View reviewed changes
Copy link
Member

@geofjamg geofjamg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new type is a weak way to model ground disconnectors. What about if you create a switch with ground disconnector between 2 busbar sections ? Nothing prevent to do it but it does not make any sense. A correct way to model this is to add a new equipment "Ground" that will be connected to the topology and a ground disconnector will just be a disconnector that connect a Ground to the topology.

@So-Fras
Copy link
Member Author

So-Fras commented Oct 26, 2023

Thank you @geofjamg for your remarks, I have added them to the #1131 issue.
@annetill we need to talk about it on your return!

@So-Fras
Merge branch 'main' into ground_disconnector
d4876f6 
Signed-off-by: Sophie Frasnedo <93923177+So-Fras@users.noreply.github.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

57.1% 57.1% Coverage
0.0% 0.0% Duplication

@annetill
Copy link
Member

annetill commented Nov 2, 2023 •
edited
Loading

In order to enrich the modelling choice, that is directly linked to short-circuit analysis, we have to have in mind our "RTE" modelling but also open data modeling. For example:
Screenshot 2023-11-02 at 15 02 42
The disconnector GND_DIS_1 is a ground disconnector. It is connected to a bus bar section and to a "ground" object.

@annetill annetill mentioned this pull request Dec 19, 2023
Merged
7 tasks
@annetill
Copy link
Member

Rework in #2837

@annetill annetill closed this Dec 19, 2023
@So-Fras So-Fras deleted the ground_disconnector branch September 6, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geofjamg geofjamg geofjamg requested changes

Assignees
No one assigned
Labels
January release new version XIIDM/JIIDM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@So-Fras @geofjamg @annetill